Skip to content

Extract rule: template-no-dynamic-subexpression-invocations#2467

Merged
NullVoxPopuli merged 1 commit intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-no-dynamic-subexpression-invocations
Mar 12, 2026
Merged

Extract rule: template-no-dynamic-subexpression-invocations#2467
NullVoxPopuli merged 1 commit intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-no-dynamic-subexpression-invocations

Conversation

@NullVoxPopuli
Copy link
Contributor

Split from #2371.

@NullVoxPopuli NullVoxPopuli force-pushed the nvp/template-lint-extract-rule-template-no-dynamic-subexpression-invocations branch from 0ff5816 to be6400b Compare March 12, 2026 21:34
@NullVoxPopuli NullVoxPopuli merged commit e20ba32 into ember-cli:master Mar 12, 2026
9 checks passed
@johanrd
Copy link
Contributor

johanrd commented Mar 12, 2026

Claude review comparing with ember-template-lint rule: no-dynamic-subexpression-invocations

Behavioral extension: body-context mustaches flagged

The original only flags MustacheStatement when it's in attribute position (lines 27-37):

case 'MustacheStatement': {
  let isAttr = parents.some((it) => it.node.type === 'AttrNode');
  if (isAttr && isDynamic && hasArguments) {
    this.log({ ... });
  }
}

The PR adds an extra branch that flags this.* paths in body context too:

  if (!inAttr && hasArgs) {
    if (node.path.head?.type === 'ThisHead') {
      context.report({ ... });
    }
  }

This means {{this.formatter this.data}} in body context is flagged by the PR but NOT by the original. This is new behavior that would cause false positives for users migrating from ember-template-lint. Could be fixed by match the original byr removing !inAttr block should be removed to match the original, or keep the change.

Generic error message

The original gives context-specific messages:

  • SubExpression/ElementModifierStatement: "You cannot invoke a dynamic value in the ${node.type} position"
  • MustacheStatement in attr position: "You must use the 'fn' helper to create a function with arguments to invoke"

The PR uses a single generic message for all cases: "Do not use dynamic helper invocations. Use explicit helper names instead." The original messages are more actionable — they tell the user what node type is
affected and (for attr position) suggest using fn.

Local scope tracking (minor)

The PR implements its own localScopes stack vs the original's this.scope.isLocal(). The implementation is reasonable for the eslint context and doesn't cause behavioral differences in practice — the
path.original.includes('.') check in isDynamicPath catches all multi-part paths before isLocal is ever reached. Just noting the divergence for awareness; it's not a bug.

@github-actions github-actions bot mentioned this pull request Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants